Skip to content

Replace nativeZoom of widgets with disabled autoscaling with 100 #2599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Replace nativeZoom of widgets with disabled autoscaling with 100 #2599

wants to merge 1 commit into from

Conversation

ShahzaibIbrahim
Copy link
Contributor

@ShahzaibIbrahim ShahzaibIbrahim commented Oct 6, 2025 •
edited
Loading

Few Widgets do not adhere to AUTO_SCALE_DISABLED field when set to true in Widget#setData. The proposed changes are as follow:

  • In Widget when setting DATA_AUTOSCALE_DISABLED to true we also update the nativeZoom value to 100 since it is a public field and we cannot rely on everyone calling the getter for it where we previously had a logic to return 100 in case of autoScale disablement.
  • Tracker class in some cases was using same logic as getZoom() but missing one additional detail which is present in getZoom(), that is to check if autoScale is disabled then use 100%
  • In Control class when we set the bounds then we use to check for autoScale disablement but not when setting the size or location, so that's fixed too.

Detailed Analysis

These are the widgets or widget utils that should be affected when auto scale is disabled.

org.eclipse.swt.dnd.DragSource.drag(Event)
org.eclipse.swt.dnd.DropTarget.convertPixelToPoint(int, int)
org.eclipse.swt.dnd.TableDragSourceEffect.getDragSourceImage(DragSourceEvent)
org.eclipse.swt.dnd.TableDropTargetEffect.dragOver(DropTargetEvent)
org.eclipse.swt.dnd.TreeDragSourceEffect.getDragSourceImage(DragSourceEvent)
org.eclipse.swt.dnd.TreeDropTargetEffect.dragOver(DropTargetEvent)

org.eclipse.swt.widgets.Button.Button(Composite, int)
org.eclipse.swt.widgets.Button.getCheckboxTextOffset(long)
org.eclipse.swt.widgets.Button.wmDrawChild(long, long)

org.eclipse.swt.widgets.Composite.wmNCPaint(long, long, long)

org.eclipse.swt.widgets.Control.getData(String)
org.eclipse.swt.widgets.Control.setParent(Composite)
org.eclipse.swt.widgets.Control.WM_DPICHANGED(long, long)

org.eclipse.swt.widgets.ExpandBar.computeSizeInPixels(int, int, boolean)
org.eclipse.swt.widgets.ExpandBar.drawThemeBackground(long, long, RECT)
org.eclipse.swt.widgets.ExpandBar.drawWidget(GC, RECT)

org.eclipse.swt.widgets.Menu.fixMenus(Decorations)

org.eclipse.swt.widgets.MenuItem.fixMenus(Decorations)
org.eclipse.swt.widgets.MenuItem.getMenuItemIconBitmapHandle(Image)
org.eclipse.swt.widgets.MenuItem.getMenuItemIconSelectedBitmapHandle()

org.eclipse.swt.widgets.TabFolder.drawThemeBackground(long, long, RECT)

org.eclipse.swt.widgets.Table.setCheckboxImageList(int, int, boolean)
org.eclipse.swt.widgets.Table.wmNotifyHeader(NMHDR, long, long)

org.eclipse.swt.widgets.TaskBar.createShellLink(MenuItem)

org.eclipse.swt.widgets.Text.WM_DRAWITEM(long, long)

org.eclipse.swt.widgets.Tree.calculateAndApplyIndentSize()
org.eclipse.swt.widgets.Tree.setCheckboxImageList()
org.eclipse.swt.widgets.Tree.wmNotifyHeader(NMHDR, long, long)

When should auto scale disabling function?

Whenever widget#nativeZoom or widget#getNativeZoom is called from any of the widget, the native zoom must be 100%.

@ShahzaibIbrahim ShahzaibIbrahim linked an issue Oct 6, 2025 that may be closed by this pull request
Replace nativeZoom of widgets with disabled autoscaling with 100 vi-eclipse/Eclipse-Platform#446
Open
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 6, 2025 •
edited
Loading

Test Results

  118 files  ±0    118 suites  ±0   13m 59s ⏱️ + 1m 14s
4 583 tests ±0  4 565 ✅ ±0  18 💤 ±0  0 ❌ ±0 
  330 runs  ±0    326 ✅ ±0   4 💤 ±0  0 ❌ ±0 

Results for commit 1f7b045. ± Comparison against base commit 1460543.

♻️ This comment has been updated with latest results.

@ShahzaibIbrahim ShahzaibIbrahim marked this pull request as draft October 7, 2025 08:41
@ShahzaibIbrahim ShahzaibIbrahim force-pushed the master-446 branch 2 times, most recently from 10fe424 to 29ded18 Compare October 7, 2025 13:17
@ShahzaibIbrahim ShahzaibIbrahim marked this pull request as ready for review October 7, 2025 13:31
@ShahzaibIbrahim
Replace nativeZoom of widgets with disabled autoscaling with 100
1f7b045 
The nativeZoom of some widgets currently always has the original native
zoom value, even if autoscaling is disabled for that widget such that
they behave as if their native zoom is effectively 100. With this change
the behavior of those widgets will be fixed as per autoscale disabled
value.
fedejeanne
fedejeanne reviewed Oct 10, 2025
View reviewed changes
Copy link
Member

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does one test this?

Shouldn't line 186 (in the constructor) also be adapted?

	this.nativeZoom = parent != null ? parent.nativeZoom : DPIUtil.getNativeDeviceZoom();

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Widget.java

void handleDPIChange(Event event, float scalingFactor) {
this.nativeZoom = event.detail;
this.nativeZoom = this.autoScaleDisabled ? 100 : event.detail;;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.nativeZoom = this.autoScaleDisabled ? 100 : event.detail;;
this.nativeZoom = this.autoScaleDisabled ? 100 : event.detail;

HeikoKlare
HeikoKlare reviewed Oct 13, 2025
View reviewed changes
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Widget.java

if (DATA_AUTOSCALE_DISABLED.equals(key)) {
autoScaleDisabled = Boolean.parseBoolean(value.toString());
this.nativeZoom = 100;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not take into account that autoScaleDisabled might be set to false. In that case, I would expect nativeZoom to be reset to shell zoom, i.e., nativeZoom = getData(DATA_SHELL_ZOOM). That's only available in Control but this functionality should be moved to Control anyway as it does not make sense for Widget, shouldn't it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HeikoKlare HeikoKlare HeikoKlare left review comments

@fedejeanne fedejeanne fedejeanne left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Replace nativeZoom of widgets with disabled autoscaling with 100

3 participants

@ShahzaibIbrahim @HeikoKlare @fedejeanne